Skip to content

fix(shell): the lib tests' load-dependent failures, at their causes - #10

Merged
trancong12102 merged 5 commits into
mainfrom
fix/shell-flaky-test
Oct 3, 2026
Merged

fix(shell): the lib tests' load-dependent failures, at their causes#10
trancong12102 merged 5 commits into
mainfrom
fix/shell-flaky-test

Conversation

@trancong12102

@trancong12102 trancong12102 commented Oct 3, 2026

Copy link
Copy Markdown

Reproducing it

cargo test -p gpui-shell --lib (all 712 tests, debug) passes when run alone. Run as 4 copies side by side, a load like a sync's parallel builds, it failed often:

  • 9 of the first 32 runs failed;
  • 11 of 60 failed on a second batch;
  • 4 of 48 still failed after the first round of fixes.

Six separate causes turned up. None of them is fixed by loosening a timing. Each commit addresses one cause.

test(s) that failed cause fix
websocket_sends_ordinary_and_custom_protocol_headers (7 runs) The server polls a nonblocking listener. On macOS the accepted stream inherits nonblocking mode, so its single read returned WouldBlock (35) whenever it ran before the request arrived. Read the request head whole, from the stream set back to blocking with a deadline. A new macOS test (a_stream_accepted_off_a_nonblocking_listener_does_not_wait_for_its_client) pins down the inherited mode.
websocket_write_rejects_after_the_server_closes It slept 10 ms for the client's I/O threads, and on a loaded machine the probe was still pending. Draw until the probe settles, for up to 10 s (render_until).
kills_a_command_whose_{stdout,stderr}_exceeds_the_limit, cancellation_kills_and_reaps_the_command These tests are not about the deadline, yet they ran under a 2 s one. Instrumented, the limit test showed the command had written nothing at 2 s. Tests not about the deadline use a 600 s one. Tests that check a command was cut short run sleep 600 and allow 60 s.
registered_argument_conversion_has_recursive_depth_and_aggregate_budgets The sandbox's interrupt deadline (50 ms per render) used the wall clock, so a render the OS left waiting for the CPU was interrupted. In production this fails a view because the machine is busy. The deadline now counts the thread's CPU time (metrics::thread_cpu_time). It still stops runaway scripts, which the existing infinite-loop and swallowed-interrupt tests confirm.
a_templated_panel_against_the_chain_it_replaces, what_automatic_templating_of_the_safe_helpers_would_buy (12 runs), benchmark one_recorded_call_is_priced_stage_by_stage Each times all of one side's rounds, then all of the other's, on the wall clock, so load on one side alone reversed the comparison (measured once at 0.53x where 1.24x is normal). Rounds alternate between the two sides, and each side is timed on the thread's CPU clock.
a_cached_mirror_with_the_wrong_origin_is_refused A local-path dependency was mirrored by a local clone, which copies the source's objects/ file by file. The fixture's commit leaves a background git maintenance running, and its maintenance.lock vanished mid-copy. Mirrors are cloned with --no-local. The cache's own git commands run with maintenance.auto=false and gc.auto=0, so no detached maintenance is left inside mirrors the cache renames and clones from. New test: a lock planted in the source never reaches the mirror. It failed before the fix.

Result

The same 4-copy load ran 60 of 60 green, and a 6-copy load ran 48 of 48 green.

Other checks:

  • cargo test -p gpui-shell --lib: 713 passed.
  • fmt is clean.
  • Clippy reports nothing in gpui-shell's own code apart from pre-existing too_many_arguments (7/6) errors under the local clippy 1.99. I checked it with --no-deps -A clippy::too_many_arguments.

The redirect-target server in websocket_redirect_does_not_escape_the_authorized_host still uses 50 accept tries of 10 ms. Under load that can only make it miss a wrongful connect, which would be a false pass, never a false failure; it is left as is.

Upstream checked first: longbridge/gpui-kit longbridge#3303 fixes flaky drag tests in text_selection, which is unrelated; there is no open gpui-shell flake PR.

🤖 Generated with Claude Code

trancong12102 and others added 5 commits October 4, 2026 01:15
@trancong12102 @claude
test(shell): wait for the WebSocket peers instead of racing them
a4fba84 
Two network tests failed under load.

- The headers test polls a nonblocking listener for its client, and on
  macOS the accepted stream inherits the mode, so its one read failed with
  WouldBlock whenever the server read before the request arrived. It now
  reads the request head whole from the stream set back to blocking, with a
  deadline; a macOS test pins down the inherited mode.
- The closed-socket test slept 10 ms for the client's I/O threads; the probe
  was still pending on a loaded machine. It now draws until the probe
  settles, for at most ten seconds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trancong12102 @claude
test(shell): give process tests a deadline they are not testing
52fb484 
The output-limit, capture and cancellation tests ran their commands under
a 2 s deadline, and the cancellation and pipe tests required an answer
within 1 s of a 5 s sleep. Under load a command could write nothing in
2 s, so the limit test reported a timeout. The tests that are not about
the deadline now use one of ten minutes, and the ones that check a
command was cut short run a 600 s sleep and allow 60 s for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trancong12102 @claude
fix(shell): budget script work on the thread's CPU time
f95a5db 
The interrupt deadline measured the wall clock, so a render the machine
left waiting for the CPU ran out of its 50 ms and failed the view though
the script had not kept the interpreter that long: the argument-budget
render test was interrupted that way under load. The deadline now counts
the thread's CPU time (`metrics::thread_cpu_time`, the wall clock where
no thread clock is read), so it still cuts off a runaway script and no
longer cuts off a busy machine.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trancong12102 @claude
test(shell): compare timings on the CPU clock with interleaved rounds
5e661aa 
Three tests assert one piece of work costs less than another: templating
against the builder chain, templated leaf helpers against plain ones, and
the dedicated style entry point against the generic one. Each timed all of
one side's rounds and then the other's on the wall clock, so load during
one side alone reversed the comparison (templated helpers once measured
0.53x). The rounds now alternate between the sides, and each is timed on
the thread's CPU clock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trancong12102 @claude
fix(shell): clone dependency mirrors without copying or maintenance
66b5e2f 
A Git dependency at a local path was mirrored by a local clone, which
copies the source's object directory file by file. The fixture's commit
left a background `git maintenance` in the source, whose
`maintenance.lock` vanished mid-copy and failed the clone. The mirror is
now cloned with `--no-local`, through Git's transport. The cache's own
Git commands run with `maintenance.auto=false` and `gc.auto=0`, so none
leaves maintenance running in a mirror it renames and clones from. The
new test plants a lock in the source and finds none in the mirror.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trancong12102
trancong12102 merged commit 9b97cdd into main Oct 3, 2026
7 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@trancong12102